annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java (1)

557-578: Consider documenting the shared mutable state in IncludeContext.

The record's includeStack is intentionally shared across all withSourceFile() copies for cycle detection. Since records are typically expected to be immutable, a brief comment clarifying this design choice would help future maintainers.

📝 Suggested documentation

+    /**
+     * Context for resolving include paths during YAML parsing.
+     * <p>
+     * Note: The {`@code` includeStack} is intentionally shared across all instances
+     * created via {`@link` `#withSourceFile`(Path)} to track the include chain for
+     * cycle detection during recursive parsing.
+     */
     private record IncludeContext(Path sourceFile, Path basePath, Deque<Path> includeStack) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java`
around lines 557 - 578, The IncludeContext record uses a shared, mutable Deque
includeStack intentionally for cycle detection; add a concise comment inside the
IncludeContext declaration explaining that includeStack is shared across copies
(including those returned by withSourceFile) by design, why it must remain
mutable for include-cycle detection, and that withSourceFile intentionally
preserves the same includeStack instance rather than copying it to avoid
breaking that logic; reference IncludeContext, includeStack, and withSourceFile
in the comment so future maintainers understand this deviation from usual record
immutability expectations.

core/src/main/java/com/predic8/membrane/core/router/IncludeList.java (2)

9-11: Complete the TODO placeholders in Javadoc.

The @description TODO placeholders at lines 10 and 18 should be replaced with meaningful descriptions before merging. This documentation is used for generating user-facing configuration references.

Also applies to: 17-19

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/src/main/java/com/predic8/membrane/core/router/IncludeList.java` around
lines 9 - 11, Replace the placeholder Javadoc `@description` TODO entries in the
IncludeList class with meaningful documentation: update the class-level Javadoc
for class IncludeList to describe its purpose (what an include list config
represents and how it's used), and update the second Javadoc block (the one tied
to the include-list element or related getter/setter) to explain the
configuration semantics (what entries mean, expected format, and any
defaults/behavior). Edit the Javadoc comments around the IncludeList class and
its public API (e.g., the class declaration and the include-list
accessor/mutator) to provide concise, user-facing descriptions used for
configuration reference.

---

15-27: Consider encapsulation improvements for the includes field.

The field lacks an explicit access modifier (defaulting to package-private) and the getter/setter expose the internal list directly, allowing external modification.

♻️ Proposed fix for encapsulation

-    List<String> includes = new ArrayList<>();
+    private List<String> includes = new ArrayList<>();

     /**
      * `@description` TODO
      */
     `@MCChildElement`(allowForeign = true)
     public void setInclude(List<String> include) {
-        this.includes = include;
+        this.includes = include != null ? new ArrayList<>(include) : new ArrayList<>();
     }

     public List<String> getInclude() {
-        return includes;
+        return new ArrayList<>(includes);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/src/main/java/com/predic8/membrane/core/router/IncludeList.java` around
lines 15 - 27, Make the includes field private and prevent external code from
mutating the internal list: change the field declaration for includes to
private, update setInclude(List<String> include) to assign a defensive copy
(e.g., new ArrayList<>(include) or Collections.emptyList() for null), and update
getInclude() to return either an unmodifiable view
(Collections.unmodifiableList(includes)) or a defensive copy (new
ArrayList<>(includes)) so callers cannot modify the internal state directly;
keep the method names setInclude and getInclude as-is.

annot/src/main/java/com/predic8/membrane/annot/generator/IncludeListClassGenerator.java (1)

61-63: Normalize and defensively copy include input.

Line 62 stores caller state by reference and allows null. This can leak mutability or push null-handling downstream. Prefer copy + null normalization.

Proposed patch

                 `@MCChildElement`(allowForeign = true)
                 public void setInclude(List<String> include) {
-                    this.includes = include;
+                    this.includes = include == null ? new ArrayList<>() : new ArrayList<>(include);
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@annot/src/main/java/com/predic8/membrane/annot/generator/IncludeListClassGenerator.java`
around lines 61 - 63, The setInclude method currently assigns the caller's List
reference directly to the includes field and allows null; change setInclude in
IncludeListClassGenerator to defensively copy and normalize the input by setting
this.includes to an empty list when include is null and otherwise to a new
ArrayList<>(include) (optionally wrap with Collections.unmodifiableList(...) if
immutability is desired) so caller state is not leaked and nulls are normalized.